fix: drain stale futures in __get_prop_handler on batch failure#1723
Open
vergil-zhao wants to merge 1 commit into
Open
fix: drain stale futures in __get_prop_handler on batch failure#1723vergil-zhao wants to merge 1 commit into
vergil-zhao wants to merge 1 commit into
Conversation
When get_props_async raises (cloud timeout, network error, etc.) the pending futures registered in self._get_prop_list stay unresolved forever. Subsequent get_prop_async calls with the same (did, siid, piid) key reuse the dead future via the cache lookup at the top of get_prop_async, so the integration never recovers from a single batch failure until Home Assistant is restarted. This was reproducible whenever the cloud HTTP request hung past the caller's wait_for timeout: every later poll for that key returned the same cancelled-but-still-pending future. Property entities driven by async_update silently stopped updating with no error in the logs. Wrap the await in try/except and fall through with results=[]. The existing fallback loop then drains every key still in props_req with set_result(None), unblocking awaiters and freeing the cache slot so the next aggregation cycle starts fresh.
Author
|
Hit this in production today on a PTX 透明物联 three-phase breaker (cloud-poll path, exactly as described). Concrete numbers in case helpful for triage:
Independently arrived at the same diff before finding this PR — confirming both the diagnosis and that this is the right fix. Any chance of a re-review ping? Happy to address feedback or sign whatever's needed if anything's blocking. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When the cloud batch request inside
__get_prop_handlerraises (timeout, network error, transient HTTP failure), the pending futures registered inself._get_prop_liststay unresolved forever. Becauseget_prop_asynclooks up_get_prop_list[key]at the top andreturn await prop_obj['fut']if found, every subsequent call with the same(did, siid, piid)reuses the dead future — the integration never recovers from a single batch failure until Home Assistant is restarted.Reproduction
get_prop_asyncperiodically (e.g. anasync_updateoverride on a property entity, orhomeassistant.update_entitytriggered from an automation).asyncio.wait_for(..., timeout=N)shorter than__mihome_api_post_async's 30 s timeout — common when working around HA's 10 s "entity update is taking over 10 seconds" warning.wait_forcancels, but the underlying future created at line 800 stays pending. From that moment on, every poll for that key resolves the same dead future → entity values silently freeze with no error in the logs.In the affected integration the only path back to a working state is a HA restart that clears
_get_prop_list.Fix
Wrap the
await self.get_props_async(...)call intry/exceptand fall through withresults = []. The existing fallback loop right below already iterates every key still inprops_req, pops it from_get_prop_list, and callsset_result(None)— so reusing that path drains all stale futures and frees the cache slots. The next aggregation cycle starts fresh.The change is contained to one block in
miot/miot_cloud.py::__get_prop_handler. No public surface, return value, or call site changes. The broad-except + lazy_LOGGER.errorformatting follow the existing convention used elsewhere inmiot_lan.py/miot_network.py.Verification
pylint --rcfile=.pylintrc custom_components/xiaomi_home/miot/miot_cloud.py→ 10.00/10Possibly related issues
These open issues describe the exact symptom this fix addresses ("data stops updating, reload/restart restores it"), though they likely have multiple root causes — this PR only covers the cloud-batch-failure → future-leak path:
get_prop_async)